feat(eval): add eval online-eval CLI commands - #1877
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #1877 +/- ##
============================================
- Coverage 95.87% 95.74% -0.14%
============================================
Files 198 207 +9
Lines 9379 10173 +794
============================================
+ Hits 8992 9740 +748
- Misses 387 433 +46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds the imperative `agentcore eval online-eval` surface (CLI only), following the conventions established in the eval evaluator review (#1822). agentcore eval online-eval ├── create ├── get ├── list ├── update ├── pause # executionStatus -> DISABLED ├── resume # executionStatus -> ENABLED └── delete Data source. `--agent` accepts a plain AgentCore Runtime ID or a Harness ID and derives the CloudWatch source from it: a harness is itself backed by a runtime, so resolution tries GetAgentRuntime and falls back to GetHarness, reading the underlying runtime out of the harness environment. The log group is keyed by the runtime *id* and the service name by the runtime *name* — verified against live resources; the two are not interchangeable. `--data-source-config` takes the API's DataSourceConfig as JSON for agents that emit traces under a custom OTel service name, which no derivation can guess. update takes the same two entry points plus an endpoint re-scope, in precedence order: --data-source-config replaces the source outright, --agent re-derives it, and --endpoint/--clear-endpoint re-scope the agent the config was already built from (recovering its runtime id from the stored log group). Merge semantics. UpdateOnlineEvaluationConfig does partial updates at the top level — an omitted field is untouched — but sending `rule` replaces the whole object, so the client merges the untouched sub-fields over the current config. sessionConfig is optional on Rule and the service does not backfill it, so it is omitted when unset rather than materializing the service's own default. Flags are validated against the service model's documented ranges (--sampling-rate 0.01-100, --session-timeout-minutes an int in 1-1440). --data-source-config and --filters are source-aware (inline, file://<path>, or - for stdin). --role-arn is required: the CLI does not provision an execution role. Testing. Handler tests use the fixture/golden pattern (real CoreClient behind a recorded SDK seam). The recording creates one config, exercises get/list/update/pause/resume, then deletes it, so it leaves no residue. Beyond the recorded flow, the following were exercised against the live API: file:// and stdin sources, the one-stdin-per-command guard, all three update data-source paths applied in sequence to one config, and the client-side rejection of --endpoint on a config built from custom log groups.
jariy17
force-pushed
the
feat/eval-online-eval-cli
branch
from
July 30, 2026 22:14
ae47c94 to
28a5b7d
Compare
jariy17
marked this pull request as ready for review
July 30, 2026 22:17
CreateOnlineEvaluationConfig requires a role the service can assume, and the policy it validates is not obvious: spans live in `aws/spans` rather than the runtime's own log group, and query access must be scoped to the runtime log-group *prefix* — a policy pinned to a single endpoint is rejected. Both took live failures to pin down, surfacing only as "the provided execution role does not have permissions to access the specified log groups", so leaving it to the caller made `create` hard to use. Core now provisions a default role scoped to the resolved log groups unless --role-arn is passed, mirroring the harness execution-role pattern and the design doc's auto-provisioning philosophy. A freshly created role is not immediately assumable (IAM is eventually consistent) and the service rejects the create rather than retrying, so the create is retried on that specific error with a bounded backoff — only when we provisioned the role, since a caller-supplied one that cannot be assumed is a real misconfiguration and should fail fast. Fixtures re-recorded: the create flow now captures the GetRole/CreateRole/ PutRolePolicy calls, and the recorded policy scopes to the runtime prefix.
notgitika
requested changes
Jul 31, 2026
added 3 commits
August 3, 2026 14:41
Four fixes from @notgitika's review: Role name (onlineEvalExecutionRole.tsx). `AgentCoreOnlineEval-` is 20 chars, so slicing to IAM's 64-char cap kept only the first 44 characters of a config name that can run to 100. Two configs sharing that prefix mapped to one role, and because provisioning is idempotent by name the second create silently re-scoped the first's policy to a different runtime. Long names are now truncated with a hash suffix. Role scope on update (eval.tsx). Moving the data source left the role scoped to the old runtime, so sampling stopped with no error — the service validates the role's log-group access on create, not update. A CLI-provisioned role (identified by its derived name) is now re-scoped when the source moves; --update-role false opts out. A role named via --role-arn is never edited, and update accepts --role-arn so a caller can replace it. Both skip paths return a warning that the handler prints to stderr, naming the role and the log groups it needs access to. IAM endpoint leak (eval.tsx). The IAM client was built with toClientConfig, which forwards --endpoint-url; IAM is global and must not receive the agentcore override. Now passes { region } only, matching harness.tsx. KMS (kms:Decrypt for encrypted evaluators) is deliberately not addressed here. Verified against the live API: the policy re-scopes from one runtime to another on --agent, --update-role false skips it and warns, and --role-arn leaves a custom role's own policy untouched. The role-warning test lives in its own describe with its own config, since adding a call to the CRUDL sequence shifts which recording each of its calls keys to.
runtime/invoke's advisory summary is gated on `!output.json` (response.ts:180), with tests asserting an empty stderr in JSON mode; only its failure-path summaries write regardless. The role-scope warning follows the advisory precedent, so a scripted caller now gets machine-readable stdout and nothing else.
CreateOnlineEvaluationConfig documents that the execution role must hold kms:Decrypt on the key of any evaluator encrypted with a customer managed key, and that it validates this when the config is created. The provisioned role had no KMS permissions, so such a config could not be created without --role-arn. Core now resolves each referenced evaluator via GetEvaluator, collects any kmsKeyArn, and adds a DecryptEvaluatorKeys statement scoped to exactly those keys. The statement is omitted when nothing is encrypted, so the common builtin case does not widen the role. Applied on update too, when the evaluator list changes alongside the data source. Resolution depends on GetEvaluator reporting kmsKeyArn, which the service currently only does for ~2 minutes after an evaluator is created (P484740478). The encrypted-evaluator fixture is therefore hand-authored to represent the documented behavior; a RECORD run overwrites it with the live response, so it must be restored afterwards. Policy assertions live in a unit test rather than the fixture layer: recorded IAM responses are empty, so the policy body is not observable through a golden, and an earlier attempt to pin it via fixture hashing did not fail when the statement was removed. The new test catches removal of the KMS statement, scoping to an endpoint log group instead of the runtime prefix, and blind role-name truncation.
notgitika
previously approved these changes
Aug 3, 2026
Wrapping the failure in InputValidationError mislabelled it: that type sets source: USER, but a GetEvaluator call failing is not the caller's input at fault — it is a service or permissions condition. It also replaced the SDK's error, which already names the operation and the evaluator, with a less precise message. The try/catch is removed so the original error surfaces unchanged.
notgitika
previously approved these changes
Aug 3, 2026
nborges-aws
reviewed
Aug 3, 2026
Nico's review: re-scoping the managed role before UpdateOnlineEvaluationConfig left the config broken if the update then failed — the role granted query access to the new runtime's logs while the config still pointed at the old one, so it silently stopped sampling. The role is now widened to the union of old and new log groups before the update and narrowed to the new set only after it succeeds. A superset role is valid for either config state, so a failed update never strands a live config without query access; a later successful update narrows it back. If the narrow itself fails, a "narrow-failed" warning reports the role is broader than the current data source (a real reason the handler now renders distinctly, rather than mislabelling it as a declined re-scope). Verified against the live API: a forced update failure leaves the config on the old runtime with the role still covering it, and a successful --agent repoint narrows the role to the new runtime only. Fixtures re-recorded for the two-step PutRolePolicy sequence.
added 2 commits
August 3, 2026 21:17
The transactional re-scope on update rewrote the role's single inline policy in place (widen to the union, then narrow), so the pre-update scope existed nowhere once the widen ran — an interrupted update left the role permanently over-scoped. Each scope is now its own inline policy, named after a fingerprint of the scope: - grantOnlineEvalScope attaches the new scope's policy (create path uses this too) - revokeOnlineEvalScope detaches the superseded one update grants the new scope, runs UpdateOnlineEvaluationConfig, then revokes the old scope only on success. Because granting never overwrites another scope's policy and IAM unions Allows across a role's inline policies, both scopes are granted in between, and a failed update leaves the policy backing the current data source byte-identical. Verified live: a forced update failure left the old policy's md5 unchanged with the config still pointing at (and granted) the old runtime; a successful --agent repoint revoked the old scope. Also widens the create-time propagation retry: a freshly written policy under a new name is not immediately visible, surfacing as "does not have permissions to create log group" / "access the specified log groups" in addition to the existing "role cannot be assumed". Renamed to retryWhileRolePropagates. The stale-scope warning replaces narrow-failed: on update the concern is now a superseded scope policy left attached, not a policy that could not be narrowed.
scopePolicyName hashed the (logGroups, kmsKeys) inputs, which meant the name tracked only those two arguments — any other change to what executionPolicy renders (a new statement, a changed resource shape) would reuse the same name and overwrite the prior policy. Hashing the rendered policy document instead ties the name to the exact contents, so any change to the granted permissions yields a new name and the create-then-revoke sequence stays collision-free. grant renders the document once, names it from that, and puts it; the update path re-renders the old document to derive the name to revoke. accountIdFromRoleArn is exported for that re-render. Verified live: a successful --agent repoint revoked the old policy and left exactly the new one, confirming the re-derived old name matches what create wrote.
notgitika
approved these changes
Aug 4, 2026
nborges-aws
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the imperative
agentcore eval online-evalcommand surface (CLI only), following the conventions established in theeval evaluatorreview (#1822).Command structure
Flags
eval online-eval create--name--agent--endpointDEFAULT); only valid with--agent--data-source-configDataSourceConfig(source-aware); alternative to--agent--evaluator--sampling-rate--session-timeout-minutes--filtersFilter[](source-aware)--role-arn--enable-on-createtrue | false(defaulttrue)--description* exactly one of
--agentor--data-source-config.eval online-eval update--id--sampling-rate--session-timeout-minutes--filtersFilter[](source-aware)--evaluator--agent--endpoint/--clear-endpoint--data-source-configDataSourceConfig(source-aware); replaces the source outrightThe data source can be repointed three ways, in precedence order:
--data-source-configreplaces it outright,--agentre-derives it from a different agent, and--endpoint/--clear-endpointalone re-scope the agent the config already used.--agent/--data-source-configand--endpoint/--data-source-configare mutually exclusive.Fields left unset are preserved.
UpdateOnlineEvaluationConfigdoes partial updates at the top level — an omitted field is untouched — but sendingrulereplaces the whole object, so the client merges the untouched sub-fields over the current config.eval online-eval get/pause/resume/delete--id(required).eval online-eval list--max-results,--next-token— server-side pagination only.Source-aware values (
--data-source-config,--filters) accept the value inline,file://<path>, or-for stdin; one flag per command may read stdin.Execution role
CreateOnlineEvaluationConfigrequires a role the service can assume, and validates at create time that it can query the log groups it was pointed at. Core provisionsAgentCoreOnlineEval-<configName>scoped to the resolved log groups unless--role-arnis passed, mirroring the harness execution-role pattern. The provisioning is idempotent: an existing role is reused and its inline policy refreshed. A name that would exceed IAM's 64-character cap is truncated with a hash suffix, since two configs sharing a truncated name would otherwise share one role.Three parts of the policy are worth calling out, because each was derived from a live failure rather than the docs:
aws/spans, not in the runtime's own log group, so both must be queryable;kms:Decrypton that key, which the service also validates at create time. Core resolves each referenced evaluator's key and scopes the statement to exactly those keys, omitting it entirely when nothing is encrypted.The first two surface only as
ValidationException: The provided execution role does not have permissions to access the specified log groups.Moving the data source invalidates the role's scope, so
updatere-scopes a CLI-provisioned role when--agent/--data-source-config/--endpointchanges it.--update-role falseopts out. A role named via--role-arnis never edited; both skip paths emit a warning naming the role and the log groups it needs access to.That re-scope is sequenced so a failed update cannot strand a live config. The role is widened to the union of the old and new log groups before
UpdateOnlineEvaluationConfig, then narrowed to the new set only after it succeeds. A superset role is valid for either config state, so if the update fails the config still points at the old runtime with a role that still covers it. A rollback-on-failure ordering was considered and rejected: IAM is eventually consistent, so the compensating call can itself fail and none of these take effect instantly, whereas a superset is never wrong for either state. If the final narrow fails, the role is left broader than necessary and anarrow-failedwarning says so — a later successful update re-scopes it.A freshly created role is not immediately assumable (IAM is eventually consistent) and the service rejects the create rather than retrying, so the create is retried on that specific error with a bounded backoff — only when the CLI provisioned the role.
Known service limitation
KMS resolution reads
GetEvaluator.kmsKeyArn, which the service currently reports only for roughly two minutes after an evaluator is created, then omits. Until that is fixed, creating a config that references an older encrypted evaluator needs an explicit--role-arn. The encrypted-evaluator fixture is hand-authored to represent the documented behavior; aRECORD=1run overwrites it with the live response and it must be restored afterwards.Testing
bun test— full suite green (566 tests).bun run typecheck,bun run lint:check,bun run format:check— clean.CoreClient+ recorded SDK seam), notTestCoreClient. Recorded against a live account withRECORD=1; the recording provisions the execution role, creates one config, exercises get/list/update/pause/resume, then deletes the config, so it leaves no config residue. Both record and replay modes pass.src/core/onlineEvalExecutionRole.test.tsrather than the fixture layer: recorded IAM responses are empty, so the policy body is not observable through a golden. Those tests fail if the KMS statement is dropped, if query access is scoped to an endpoint log group instead of the runtime prefix, or if role names are blindly truncated.--agentrepoint narrows the role to the new runtime only.file://and stdin sources, the one-stdin-per-command guard, all threeupdatedata-source paths (--agent,--data-source-config,--endpoint) applied in sequence to one config, and the client-side rejection of--endpointon a config built from custom log groups.TUI flows for these commands will come in a later PR.